Issue #366: make mask method available to MetaSWAP model class#1871
Issue #366: make mask method available to MetaSWAP model class#1871rleander73 wants to merge 4 commits into
Conversation
|
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
I have some comments on the public API, after which we have a very useful method!
| # Check that the mask has been applied correctly to each package | ||
| for pkgname, pkg in msw_model.items(): | ||
| if isinstance(pkg, msw.meteo_mapping.PrecipitationMapping): | ||
| continue # Skip PrecipitationMapping package for this test |
There was a problem hiding this comment.
Why are we skipping the PrecipitationMapping? The actual mappings are derived upon writing, if I recall correctly, so I think these objects should be identical? Or am I missing something?
|
|
||
| Parameters | ||
| ---------- | ||
| msw_active: MetaSwapActive, dictionary of xr.DataArray |
There was a problem hiding this comment.
MetaSwapActive currenlty is a developer utility, not public API.
Adding extra dataclasses to the API adds extra entry barriers for users, which I like to avoid: They need to import additional python objects, which they need to look up in the docs.
The "all" mask usually is an .any(dim="subunit"), so I think we can derive that for the user, I would do something like:
def mask_all_packages(self, mask: GridDataArray):
...
mask_all = mask.any(dim="subunit")
msw_active = MetaSwapActive(per_subunit=mask, all=mask_all)
...| def mask_all_packages( | ||
| self, | ||
| msw_active: MetaSwapActive, | ||
| # ignore_time_purge_empty: bool = False, |
There was a problem hiding this comment.
Remove this outcommented code
3 participants
Fixes #366
Description
Checklist
Issue #nr, e.g.Issue #737pixi run generate-sbomand committed changes